Skip to content

docs: tier review round 1 by surface, drop forced minimum rounds - #1181

Merged
vivek7405 merged 2 commits into
mainfrom
feat/shorten-review-loop
Jul 30, 2026
Merged

docs: tier review round 1 by surface, drop forced minimum rounds#1181
vivek7405 merged 2 commits into
mainfrom
feat/shorten-review-loop

Conversation

@vivek7405

@vivek7405 vivek7405 commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Closes #1180

The self-review loop still charged every PR a fixed minimum of three sequential reviewer stages after #1159 and #1173: a deep-review round 1 (7 to 24 agents, whatever the PR touched), a forced second round even when deep-review confirmed nothing, and a standalone prose pass. This shortens the loop while keeping the adversarial depth where a bug actually ships into end-user apps.

What changed

In .claude/skills/webjs-start-work/SKILL.md, ### How the loop works:

  • Round 1 is tiered by the diff's paths, a path check, never a judgment call. A diff touching shipped source (packages/*/src/, packages/cli/lib/, packages/cli/templates/) takes the deep-review workflow as round 1, unchanged. Every other diff (docs, website, blog, skills, hooks, tests-only, examples) takes ONE broad fresh reviewer using the delta-verifier template with the full diff as its question. A mixed diff takes the deep tier (one shipped-source path is enough), the user asking for a deep review always forces it, and a scope change that pulls shipped source into a light-tier PR re-runs round 1 at the deep tier. The tier is path-based for the same reason the substantive gate is surface-based: membership is checkable from the diff, so the agent motivated to go light cannot misjudge it. This supersedes docs: make the self-review loop fast, parallel, and stall-proof #1159's uniform-round-1 rule at the owner's direction; the misjudgment concern that rule addressed stays closed because the tier reads paths, not importance.
  • The minimum is ONE round. A round 1 of either tier that finds nothing substantive converges the loop by itself: on the deep tier its findings were already adversarially jury-refuted, and on the light tier nothing in the diff is shipped source, so the old two-round minimum added a round to exactly the cleanest case. A fix still always buys a fresh delta round.
  • The converging round's prose-tier findings replace the standalone prose pass. Every reviewer already reports and tags prose findings alongside substantive ones, so the round that finds nothing substantive IS the prose sweep: its prose fixes apply without re-review and the loop ends. The gate's exception survives: a prose fix that touches a substantive surface gets one delta-scoped check of that fix alone.
  • The refuter gate, liveness and watchdog rules, worktree safety, posting mechanics, the substantive gate and its asymmetric tag verification, the 5-round budget, the BLOCKED and OVER BUDGET exits, the deferral ledger, and the deferred suites plus CI read after convergence are all untouched except where they cross-referenced the removed pass or minimum.

In .claude/workflows/deep-review.js: default maxAgents drops 24 to 16 (still clamped 8 to 60, still overridable per run), dynamic lenses cap at three instead of six (scout prompt, schema maxItems, both slices), and whenToUse scopes round 1 to shipped-source diffs. The six fixed lenses, the model split, the jury sizes, and every fail-open path are unchanged.

In .claude/hooks/route-skills.sh: the code-review directive now states the tiered round 1 and the new exit, staying in lockstep with the skill.

In test/hooks/review-loop-exit.test.mjs: the guard assertions pinning the prose pass and the two-round minimum are replaced by assertions pinning the tier rule, the one-round minimum, the converging-round prose application, and the trimmed workflow defaults, so a revert of any piece reds the test.

What a loop costs now

A docs or skill PR: one broad reviewer when clean, plus a delta round per fix. A shipped-source PR: deep-review at 16 agents, plus a delta round per fix. Nothing pays a forced extra round or a separate prose reviewer anymore.

Test plan

  • node --test test/hooks/review-loop-exit.test.mjs test/hooks/route-skills.test.mjs: 20/20 pass (the review-loop guard, and the router test including "every skill the hook can route to is committed in-repo").
  • Counterfactual: the guard test's new assertions fail against the pre-change files (they pin The minimum is ONE round, the tier bullet, the 16-agent default, and the absence of the standalone prose pass in either spelling). Verified whole-file against the parent commit and hunk-level for the skill's failure-handling hunk and the hook's comment hunk, both red when reverted alone.
  • deep-review.js validity in the async workflow context is covered by the existing guard test ("stays valid in the async workflow context").
  • Both copies of the skill (in-repo and ~/.claude) verified byte-identical with diff -q after the change.

Doc surfaces

Tests: the guard tests above. Markdown / docs site / scaffold / MCP / editor plugins / marketing / dogfood apps / version bumps: N/A because nothing in packages/*/src or any user-facing surface changed; this edits the Claude Code skill, the committed review workflow, and the routing hook, which ship to agents working this repo, not to anything the framework publishes.

The self-review loop charged every PR a fixed minimum of deep-review
(7 to 24 agents), a forced second round even after a clean deep-review,
and a standalone prose pass. Round 1 is now tiered by the diff's paths
(deep-review only when shipped source is touched, one broad reviewer
otherwise), a clean round 1 converges the loop by itself, and the
converging round's prose-tier findings are applied without re-review
in place of the separate prose pass. deep-review defaults trim to 16
agents and three dynamic lenses. Closes #1180.
@vivek7405 vivek7405 self-assigned this Jul 30, 2026

@vivek7405 vivek7405 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went through the whole loop section end to end with the new tier in mind. The shape holds together, but the guard test has a real observability hole, and two spots still talk like the old loop. One more thing that sits on an unchanged line so I am noting it here rather than inline: in the liveness rules (SKILL.md, the returns-without-reviewing bullet), the expected-result-shape enumeration still names only two reviewer kinds, the delta verifier and a deep-review run, while the broad round-1 reviewer is now a third kind everywhere else (step 1, the template intro, the reporting section). The one enumeration that decides whether a round happened should name it too.

Comment thread test/hooks/review-loop-exit.test.mjs Outdated
Comment thread .claude/hooks/route-skills.sh
Comment thread .claude/skills/webjs-start-work/SKILL.md Outdated
The negative anchor checked only the two-word spelling, so a partial
revert reintroducing the hyphenated prose-pass reference stayed green.
Both spellings are now rejected in the skill and the hook, the new
failure-handling wording is pinned, the liveness enumeration names the
broad round-1 reviewer, and the report template drops the rounds-2+
relic. Hunk-level counterfactuals verified red for the skill and hook.

@vivek7405 vivek7405 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-checked the observability fix and its blast radius across the whole PR surface: the tightened anchors do not false-fail anywhere, the pinned failure-handling wording is literal, the reviewer-kind enumerations agree everywhere, and nothing else still speaks in the old loop shape. Nothing to flag.

@vivek7405
vivek7405 marked this pull request as ready for review July 30, 2026 06:43
@vivek7405
vivek7405 merged commit c6e53c7 into main Jul 30, 2026
10 checks passed
@vivek7405
vivek7405 deleted the feat/shorten-review-loop branch July 30, 2026 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shorten the self-review loop: tier round 1, drop forced minimum rounds

1 participant